Skip to content

Update API names for NSMutableArray #1315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 16, 2017
Merged

Update API names for NSMutableArray #1315

merged 1 commit into from
Nov 16, 2017

Conversation

bubski
Copy link
Contributor

@bubski bubski commented Nov 11, 2017

No description provided.

@spevans
Copy link
Contributor

spevans commented Nov 11, 2017

@swift-ci please test

@bubski
Copy link
Contributor Author

bubski commented Nov 12, 2017

@spevans fixed the unit test.
Sorry about that

@pushkarnk
Copy link
Member

@swift-ci please test

@ianpartridge
Copy link
Contributor

@spevans test_lockWait() failed...?

@ianpartridge
Copy link
Contributor 

Test Case 'TestNSLock.test_lockWait' started at 2017-11-13 10:35:08.347
Fatal error: Can't form Range with upperBound < lowerBound: file /home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux/swift/stdlib/public/core/ClosedRange.swift, line 474
Current stack trace:
0    libswiftCore.so                    0x00007fec3750ed00 _swift_stdlib_reportFatalErrorInFile + 221
1    libswiftCore.so                    0x00007fec374ad2df <unavailable> + 5173983
2    libswiftCore.so                    0x00007fec371408eb <unavailable> + 1583339
3    libswiftCore.so                    0x00007fec374ad112 <unavailable> + 5173522
4    libswiftCore.so                    0x00007fec374ad077 <unavailable> + 5173367
5    libswiftCore.so                    0x00007fec371408eb <unavailable> + 1583339
6    libswiftCore.so                    0x00007fec37421e4f <unavailable> + 4603471
7    libswiftCore.so                    0x00007fec37421d67 <unavailable> + 4603239
8    libswiftCore.so                    0x00007fec371408eb <unavailable> + 1583339
9    libswiftCore.so                    0x00007fec37140380 _fatalErrorMessage(_:_:file:line:flags:) + 118
10   TestFoundation                     0x000055629f80f13b <unavailable> + 1171771
11   TestFoundation                     0x000055629f80edf7 <unavailable> + 1170935
12   TestFoundation                     0x000055629f813605 <unavailable> + 1189381
13   TestFoundation                     0x000055629f8173ac <unavailable> + 1205164
14   TestFoundation                     0x000055629f81755d <unavailable> + 1205597
15   TestFoundation                     0x000055629f8187be <unavailable> + 1210302
16   TestFoundation                     0x000055629f8106ed <unavailable> + 1177325
17   libdispatch.so                     0x00007fec3779c9d7 <unavailable> + 317911
18   libdispatch.so                     0x00007fec377abd21 <unavailable> + 380193
19   libpthread.so.0                    0x00007fec361966ba <unavailable> + 30394
20   libc.so.6                          0x00007fec3294d7c0 clone + 109
/home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux/swift/utils/build-script-impl: line 264:  2107 Illegal instruction     "$@"

@spevans
Copy link
Contributor

spevans commented Nov 13, 2017

Thats odd since that specific test function doesn't even create any ranges. I will run it locally and see If I can reproduce

alblue
alblue reviewed Nov 13, 2017
View reviewed changes
TestFoundation/TestNSArray.swift Outdated
("test_readWriteURL", test_readWriteURL),
("test_insertObjectAtIndex", test_insertObjectAtIndex),
("test_insertObjectsAtIndexes", test_insertObjectsAtIndexes),
("test_replaceObjectsAtIndexesWithObjects", test_replaceObjectsAtIndexesWithObjects)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add a comma at the end of the list, you avoid the need to do changes like in line 49.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alblue should I update this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't necessarily have to, and I would have squashed the commit with this one rather than appending - but it is something to know for the future, if nothing else :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I avoid sqashing / ammending or changing history in any other way on anything that's open. It's harder to keep track of subsequent changes and comments loose track of the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you squash these three commits together? The first caused a test failure and the comma one standalone doesn't really benefit being on its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, makes sense to me to do so after review.
Done

@alblue
Copy link
Contributor

alblue commented Nov 13, 2017

@swift-ci please test

1 similar comment
@alblue
Copy link
Contributor

alblue commented Nov 15, 2017

@swift-ci please test

@ianpartridge
Copy link
Contributor

@swift-ci please test and merge

@swift-ci swift-ci merged commit 4e92123 into swiftlang:master Nov 16, 2017
@ikesyo ikesyo mentioned this pull request Nov 17, 2017
Merged
@ikesyo ikesyo mannequin mentioned this pull request May 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alblue alblue alblue left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bubski @spevans @pushkarnk @ianpartridge @alblue @swift-ci